-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Bump target sdk to API 35 and make the app UI compatible with edge to edge #6393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Since the toolbars have primary color as bg, we should make the status bar white
Seems like we also need to update the roboelectric version that supports API 35. I'll look for a compatible version and update the PR tomorrow :) |
Tested upload/explore/nearby, it works great! |
still a draft? |
Thanks @nicolas-raoul for testing :) |
It is not easy, I think the only way is to modify the check to trigger it (then revert before committing). Thanks a lot! :-) |
It allows the last item to sits above the navigation bar while preserving edge-to-edge appearance.
Also, refactor LocationPicker and DescriptionEdit activities to use extension functions and reduce duplication
Hi, I've been sick since Sunday and haven't been able to focus on the work. I also discovered two different issues while working on this task, and will create them today. Also, the app looks fine on API 29+, but having some issues on API 28 that I'll fix by tomorrow as soon as possible. Sorry for being late with this task. |
Hey @rohit9625 please take care and do not feel sorry or worry about being late. We still have some time, so please don't try to overstretch and pull it off while putting your health at stake. Thanks for the draft PR, I'll start going through the changelog to accelerate the review process once this PR gets ready 🙂 |
Thank you so much, @RitikaPahwa4444 — your kind words means a lot to me. I’m feeling better now and should be able to resume work and finish the task this week. 🙂 |
Upgraded core-ktx version installCompatInsetsDispatch wasn't available on current version
Hi @RitikaPahwa4444, I've made the remaining changes and the PR is ready to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR bumps the target SDK to API 35 and implements edge-to-edge UI compatibility across the app. The changes include updating build configurations, creating edge-to-edge utility functions, and applying these utilities to various activities and fragments to ensure proper handling of system bars and window insets.
- Updated target and compile SDK from 34 to 35 with corresponding dependency upgrades
- Implemented comprehensive edge-to-edge utility functions for handling window insets
- Applied edge-to-edge compatibility across all major activities and fragments
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
app/build.gradle.kts | Updated target and compile SDK to API 35 |
gradle/libs.versions.toml | Updated AGP, core-ktx, and robolectric versions |
gradle/wrapper/gradle-wrapper.properties | Updated Gradle wrapper to version 8.13 |
EdgeToEdgeUtils.kt | New utility class for handling edge-to-edge window insets |
BaseActivity.kt | Added global edge-to-edge enablement |
Multiple Activity files | Applied edge-to-edge utilities to individual activities |
Layout and fragment files | Updated UI components for edge-to-edge compatibility |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
I reviewed the suggestions, and the results are good, but not needed. |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rohit9625, the code looks good in general :) Could you please add kdoc as well for the newly added public methods? I'll start testing it.
Sure, adding them :) |
Somehow on this branch, I'm not getting the dialog that says the image exists already on Commons. Happening only on fresh install; it works as intended in the next run. Also, the app is not duplicate showing the usage but the website does: |
Okay, I'm looking into this :) |
✅ Generated APK variants! |
I found certain more issues in this branch. I guess the work is not complete. However, I couldn't understand the problem that you're facing with this branch. Could you please share a video. |
Seems like an intermittent issue even on the main branch, please ignore. |
Could be but please don't merge it now. I forgot about the SearchActivity and also to handle keyboard IME insets for screens with text inputs. Just give me half an hour. |
Yes, I didn't work on this as to do that we need to look at the upstream library if they provide 16KB page size compatible version. It's not urgent as I think the last date is in November. Should I work on this now? |
@RitikaPahwa4444, could you please help me find where this layout is handled in kotlin files? I checked at UploadActivity but couldn't find a direct reference to these bottom Input fields. ![]() |
If a TODO, would you mind updating it in the PR description to clarify the scope and raise another issue for it? Given that this was released with API 35, I was expecting it to be included 😅 I'm going through the entire changelog, so you might find more comments from my side as the deadlines aren't mentioned there. |
Yes, I will create a different issue for this task and update the description once pending work is done. |
Could you try checking UploadMediaDetailAdapter? |
No worries, I found the file. It was UploadMediaDetailsFragment. |
Description (required)
Fixes #6362
What changes did you make and why?
I have made most of the activities edge-to-edge compatible, along with ensuring backward compatibility on API < 35.
Tests performed (required)
Tested prodDebug on Samsung A14 with API level 35 and Pixel 6 with API level 34.
Screenshots (for UI changes only)
Will provide the screenshot comparisons once done with edge-to-edge